Repository navigation
install: apply overrides to the range that bun add and bun update write - #46230
Conversation
`bun add` and `bun update` write the range of the version they resolved to package.json, and `sync_lockfile` copies that range into the lockfile. If a workspace member satisfies the new range, `Lockfile::bind_workspace_link` bound the dependency to the member. It did not ask if an override replaces the dependency, so the command dropped a rule that a clean install of the same package.json applies. `bind_workspace_link` now asks `linked_workspace_path` for the version of the override, as the resolver does. The dependency keeps its package if that version does not link the member. The rows are bound after the overrides of the edited files are copied into the lockfile, because the value of an override can be a range that the command wrote.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 2 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Updated 4:02 PM PT - Oct 20th, 2026
✅ @robobun, your commit d4ad217f6f97bfe28c3bb06c9c7fdf5186374088 passed in 🧪 To try this PR locally: bunx bun-pr 46230That installs a local version of the PR into your bun-46230 --bun |
|
Status Reproduced on main (c1df64b) with the test registry of
The fix is this PR: #46230. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline findings, I also checked the bind_workspace_link signature change — sync_lockfile is its only caller, so no stale call site misses the new flag — and the deferred-binding reorder: the rows are bound only after both the catalog and override syncs, so the guard reads the overrides the command just wrote rather than the pre-edit ones.
Extended reasoning...
The change touches the lockfile workspace-link binding in src/install/lockfile.rs and the package.json write-back sync in src/install/PackageManager/package_json_write_back.rs, plus the bun-workspaces test suite; it touches no security-sensitive surface. Two confirmed findings are posted inline (the two-rules-for-one-name lockfile that frozen-lockfile rejects, and already-affected lockfiles not being repaired), so a human should weigh them.
Problem
bun updateandbun adddrop an override for the name of a workspace package. If that package satisfies the range they write,bun.lockbinds it and not the version the rule chose. Exit 0, no message. 1.4.3 is not affected.Lockfile::bind_workspace_link(src/install/lockfile.rs:2451, from install: fix regressions and older bugs found after the package manager consolidation #45307) binds every rewritten range that links a workspace package and does not ask the overrides.Fix
bind_workspace_linkbinds only iflinked_workspace_pathlinks the workspace package for the version the override gives.settle_without_manifestdecides the same way on a clean install.sync_lockfilebinds after it copies the edited overrides. An override value can be the range the command wrote ("$name").test/cli/install/bun-workspaces.test.ts(48 of 54 new cases fail on main) and 34 other install suites. Self-reviewed: 8 concerns raised, 7 addressed.Background
Package::parsestores the link in place of the range.Downsides
edit_after_resolve_slowgrows by 2,009 bytes. A command runs 20 more instructions per rewritten range and allocates oneVec. A plain install is unchanged.--frozen-lockfileinstall fails, as on 1.4.3.Notes
Repro
Registry: the test registry of
test/cli/install(no-depshas 1.0.0, 1.0.1, 1.1.0, 2.0.0).package.json:{"name":"root","workspaces":["packages/*"],"overrides":{"no-deps":"1.0.0"}}packages/m/package.json:{"name":"no-deps","version":"1.0.5"}packages/bar/package.json:{"name":"bar","dependencies":{"no-deps":"^2.0.0"}}bun install: bar gets the registry'sno-deps@1.0.0.cd packages/bar && bun update: writes"no-deps": "^1.0.0". On main bar now gets the workspace package 1.0.5, andbun.lockhas nobar/no-depsentry. With this PR bar keepsno-deps@1.0.0.bun.lockandnode_modules, thenbun install:no-deps@1.0.0on both builds.Which build fails the new cases
"no-deps"inoverrides,"bar>no-deps","no-deps"inresolutions,"no-deps": "latest"), 6 commands (update,update no-deps,update --latest,add no-depsin the workspace,update -randupdate --filter barin the root) and 2 linkers. Each case comparesbun.lockwith the one that a clean install writes.update --filter barwith the hoisted linker."$no-deps"and takes the written range. The first version of this fix kept every dependency that had a rule, and failed those four.Cells
bun addandbun updatein a workspace, 13 commands, 2 linkers, 3 combinations of member version, rule value and declared range, under a rule without a selector: 78 cells. With this PR every cell has thebun.lockof a clean install. On main 12 differ.no-deps@^1.0.0): equal to a clean install on both builds. A selector does not match a range that links a workspace package, so this PR changes nothing there.--dev,--optionaland--peerforms ofbun add, andbun updatewith a peer range (18 cells): equal to a clean install. A peer gets no copy, and it already was bound to the workspace package.What a project pays that does not hit the bug
Release builds without LTO of main (c1df64b) and of this change on that commit.
llvm-size): text 101,521,860, then 101,523,860. Data and bss are equal. By symbol (llvm-nm -S): onlypackage_json_write_back::edit_after_resolve_slowchanges, 15,505 to 17,514 bytes.sync_lockfileandbind_workspace_linkare inlined there.qemu-x86_64 -one-insn-per-tb -d exec,nochain -dfilteron the symbol ranges, two runs per build):install --lockfile-onlyon 6 fixtures (no rules, a rule for another name, a rule with a selector and 8 dependents, a rule with a selector on a member name, a rule without a selector on a member name, no workspaces):lookup_selecting,bind_row,parse_with_tag,semver_query::parse_withandtag_workspace_linksrun the same number of instructions on both builds.edit_after_resolve_slowis not entered.bun updatethat rewrites 3 ranges, none links a workspace package:edit_after_resolve_slow1,148, then 1,208.bun update --latestthat writes a range that links a workspace package, no rule for the name:edit_after_resolve_slow1,016, then 1,072.lookup_selecting87, then 116: one rule lookup for each rewritten range that links.bun.lockandbun.lockbare byte-identical between the two builds on 6 of 6 fixtures, and each build installs the other's file with--frozen-lockfile.valgrindandbloatyare not in the container. The allocation count is from the code: oneVec<DependencyID>for a command that rewrites at least one range.The case that is still wrong
"overrides": {"no-deps": "npm:a-dep@1.0.1", "no-deps@^2": "1.0.0"}, workspace package 1.0.5, bar declares^2.0.0.bun updatein bar writes^1.0.0. A clean install then givesa-dep@1.0.1.--frozen-lockfilepasses. This PR and 1.4.3-canary.1:no-deps@1.0.0, and--frozen-lockfilefails once. A plainbun installcorrects it.no-depsitself ("no-deps": "1.1.0"beside"no-deps@^2": "1.0.0") this PR gives 1.1.0, as a clean install does. main gives the workspace package.Policy
Self-review
Addressed:
resolutions,bun add,update -randupdate --filterare tested.Not taken: one decider through the resolver (see Background).
Suites run on a debug build of this branch
bun-workspaces(339),overrides,nested-overrides,catalogs,bun-install,bun-install-registry,bun-lock,bun-lockb,lockfilechecks underfrozen-lockfile-*(4 files),bun-update,bun-update-lockfile-sync,bun-update-transitive,bun-add,bun-add-filter,bun-add-catalog,bun-dedupe,bun-audit,bun-remove,bun-prune,isolated-install,hoist,bun-workspaces-self-contained, and the 10 files ofmigration/. 0 failures with a per-test timeout of 180 s. The default 5 s times out under the sanitizer build in this container.